Skip to content

Fix issue about complete pgt external#1362

Merged
Oceania2018 merged 1 commit into
SciSharp:masterfrom
danielyuan476-ctrl:master
Jun 2, 2026
Merged

Fix issue about complete pgt external#1362
Oceania2018 merged 1 commit into
SciSharp:masterfrom
danielyuan476-ctrl:master

Conversation

@danielyuan476-ctrl
Copy link
Copy Markdown

No description provided.

@qodo-code-review
Copy link
Copy Markdown
Contributor

Review Summary by Qodo

Fix PgtExternalTask model and API method signature issues

🐞 Bug fix

Grey Divider

Walkthroughs

Description
• Add [FromBody] parameter to CompletePgtExternalAsync method signature
• Make CreatedAt and UpdatedAt properties nullable DateTime
• Remove ClaimedAt property from PgtExternalTask model
• Remove unnecessary JsonIgnore attributes from datetime properties
Diagram
flowchart LR
  A["IMembaseApi Interface"] -->|"Add [FromBody] parameter"| B["CompletePgtExternalAsync"]
  C["PgtExternalTask Model"] -->|"Make nullable"| D["CreatedAt/UpdatedAt"]
  C -->|"Remove property"| E["ClaimedAt"]
  C -->|"Clean up attributes"| F["JsonIgnore removal"]

Loading

Grey Divider

File Changes

1. src/Plugins/BotSharp.Plugin.Membase/Interfaces/IMembaseApi.cs 🐞 Bug fix +1/-1

Add FromBody attribute to API method parameter

• Added [FromBody] attribute to emptyBody parameter in CompletePgtExternalAsync method
• Ensures proper HTTP request body binding for POST endpoint

src/Plugins/BotSharp.Plugin.Membase/Interfaces/IMembaseApi.cs


2. src/Plugins/BotSharp.Plugin.Membase/Models/Responses/PgtExternalCompleteResponse.cs 🐞 Bug fix +2/-8

Refactor PgtExternalTask datetime properties and cleanup

• Changed CreatedAt and UpdatedAt from non-nullable to nullable DateTime properties
• Removed ClaimedAt property from PgtExternalTask class
• Removed unnecessary JsonIgnore attributes from datetime fields
• Simplified model by removing redundant property and attributes

src/Plugins/BotSharp.Plugin.Membase/Models/Responses/PgtExternalCompleteResponse.cs


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review Bot commented Jun 2, 2026

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0)

Grey Divider


Action required

1. Wrong body attribute 🐞 Bug ≡ Correctness
Description
IMembaseApi.CompletePgtExternalAsync uses ASP.NET Core MVC [FromBody] on a Refit client
interface, so Refit may not serialize/send the request body for this POST call. If
/pgt-external/{correlationId}/complete requires a JSON body (even {}), the call can still fail
at runtime.
Code

src/Plugins/BotSharp.Plugin.Membase/Interfaces/IMembaseApi.cs[63]

Evidence
The method lives in a Refit interface and the plugin registers it as a Refit client; [FromBody] is
an MVC attribute brought in via global usings, while the rest of the interface uses Refit [Body]
for request bodies—indicating this parameter is not correctly marked for Refit body serialization.

src/Plugins/BotSharp.Plugin.Membase/Interfaces/IMembaseApi.cs[1-64]
src/Plugins/BotSharp.Plugin.Membase/MembasePlugin.cs[21-38]
src/Plugins/BotSharp.Plugin.Membase/Using.cs[8-12]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`CompletePgtExternalAsync` is part of a Refit REST client interface but its new parameter is annotated with ASP.NET Core MVC `[FromBody]`. Refit expects its own `[Body]` attribute for request-body serialization; using `[FromBody]` may result in no body being sent.

## Issue Context
This interface is registered via `AddRefitClient<IMembaseApi>(...)`, and other methods in the same interface consistently use Refit `[Body]` for payloads.

## Fix Focus Areas
- src/Plugins/BotSharp.Plugin.Membase/Interfaces/IMembaseApi.cs[62-64]

### Suggested fix
- Change `[FromBody] object emptyBody` to `[Body] object emptyBody`.
- (Optional, clearer) Replace `object` with a dedicated empty request type (e.g., `PgtExternalCompleteRequest`) and pass `new PgtExternalCompleteRequest()` (or `new {}`) from call sites.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

@adenchen123
Copy link
Copy Markdown
Contributor

reviewed


[Post("/graph/{graphId}/pgt-external/{correlationId}/complete")]
Task<PgtExternalCompleteResponse> CompletePgtExternalAsync(string graphId, string correlationId);
Task<PgtExternalCompleteResponse> CompletePgtExternalAsync(string graphId, string correlationId, [FromBody] object emptyBody);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. Wrong body attribute 🐞 Bug ≡ Correctness

IMembaseApi.CompletePgtExternalAsync uses ASP.NET Core MVC [FromBody] on a Refit client
interface, so Refit may not serialize/send the request body for this POST call. If
/pgt-external/{correlationId}/complete requires a JSON body (even {}), the call can still fail
at runtime.
Agent Prompt
## Issue description
`CompletePgtExternalAsync` is part of a Refit REST client interface but its new parameter is annotated with ASP.NET Core MVC `[FromBody]`. Refit expects its own `[Body]` attribute for request-body serialization; using `[FromBody]` may result in no body being sent.

## Issue Context
This interface is registered via `AddRefitClient<IMembaseApi>(...)`, and other methods in the same interface consistently use Refit `[Body]` for payloads.

## Fix Focus Areas
- src/Plugins/BotSharp.Plugin.Membase/Interfaces/IMembaseApi.cs[62-64]

### Suggested fix
- Change `[FromBody] object emptyBody` to `[Body] object emptyBody`.
- (Optional, clearer) Replace `object` with a dedicated empty request type (e.g., `PgtExternalCompleteRequest`) and pass `new PgtExternalCompleteRequest()` (or `new {}`) from call sites.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

@Oceania2018 Oceania2018 merged commit c5986d4 into SciSharp:master Jun 2, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants